-
Notifications
You must be signed in to change notification settings - Fork 2
Feat 36 login api #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
๐ WalkthroughWalkthroughThis pull request introduces a comprehensive authentication and group management system, including login/signup flows with multiple steps, a group/club creation wizard, and extensive new UI components. It also removes GitHub workflow automation files and adds API client integration with Zustand state management. Changes
Sequence DiagramssequenceDiagram
participant User
participant LoginModal
participant useLoginForm
participant authService
participant apiClient
participant AuthStore
participant Router
User->>LoginModal: Enters email & password
User->>LoginModal: Clicks login button
LoginModal->>useLoginForm: handleLogin()
useLoginForm->>useLoginForm: Validate email & password
useLoginForm->>authService: login(email, password)
authService->>apiClient: POST /auth/login
apiClient->>apiClient: Add Authorization header
apiClient-->>authService: LoginResponse
useLoginForm->>useLoginForm: Extract accessToken
useLoginForm->>useLoginForm: Store in cookies (js-cookie)
useLoginForm->>AuthStore: login({ email })
useLoginForm->>useLoginForm: Show toast success
useLoginForm->>Router: navigate("/")
Router-->>User: Redirect to home
sequenceDiagram
participant User
participant SignupPage
participant TermsAgreement
participant EmailVerification
participant PasswordEntry
participant ProfileSetup
participant ProfileImage
participant SignupComplete
User->>SignupPage: Lands on /signup
SignupPage->>TermsAgreement: Render Step 1
User->>TermsAgreement: Accept terms
TermsAgreement->>SignupPage: onNext() โ step = "email"
SignupPage->>EmailVerification: Render Step 2
User->>EmailVerification: Enter email & verify code
EmailVerification->>SignupPage: onNext() โ step = "password"
SignupPage->>PasswordEntry: Render Step 3
User->>PasswordEntry: Enter password
PasswordEntry->>SignupPage: onNext() โ step = "profile"
SignupPage->>ProfileSetup: Render Step 4
User->>ProfileSetup: Enter profile info (nickname, intro, name, phone)
ProfileSetup->>SignupPage: onNext() โ step = "profile-image"
SignupPage->>ProfileImage: Render Step 5
User->>ProfileImage: Upload image & select interests
ProfileImage->>SignupPage: onNext() โ step = "complete"
SignupPage->>SignupComplete: Render Step 6
SignupComplete->>User: Show completion with options (search/create meeting/continue)
sequenceDiagram
participant User
participant CreateClubWizard
participant Step1
participant Step2
participant Step3
participant Step4
User->>CreateClubWizard: Lands on /groups/create
CreateClubWizard->>Step1: Render club name & description
User->>Step1: Enter name, check duplicates, add description
Step1->>CreateClubWizard: canNext validation passes โ onNext()
CreateClubWizard->>Step2: Render image upload & visibility
User->>Step2: Upload profile image, toggle visibility
Step2->>CreateClubWizard: onNext()
CreateClubWizard->>Step3: Render category & participant selection
User->>Step3: Select up to 6 categories, participants, activity area
Step3->>CreateClubWizard: onNext()
CreateClubWizard->>Step4: Render optional SNS/links
User->>Step4: Add social links (dynamic rows)
Step4->>CreateClubWizard: Complete (submit or finish)
CreateClubWizard->>User: Club creation complete
Estimated code review effort๐ฏ 5 (Critical) | โฑ๏ธ ~120 minutes This PR introduces major new subsystems spanning authentication (login/signup with multi-step flows), group/club management (creation wizard, search), comprehensive UI component library, API client with error handling, and state management. The heterogeneous nature of changes across authentication, routing, forms, validation, and UI requires careful review of control flow, validation logic, state transitions, and integration points. Possibly related PRs
Suggested reviewers
Poem
๐ฅ Pre-merge checks | โ 2 | โ 3โ Failed checks (3 warnings)
โ Passed checks (2 passed)
โ๏ธ Tip: You can configure your own custom pre-merge checks in the settings. โจ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @shinwokkang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! ์ด PR์ ์ฌ์ฉ์ ์ธ์ฆ ์์คํ ์ ํต์ฌ ๊ธฐ๋ฅ์ ๊ตฌ์ถํ๊ณ , ์ ํ๋ฆฌ์ผ์ด์ ์ ์ ๋ฐ์ ์ธ UI/UX๋ฅผ ๊ฐ์ ํ๋ฉฐ, ๋ ์ ๋ชจ์ ๊ด๋ จ ๊ธฐ๋ฅ์ ํ์ฅํ๋ ๋ฐ ์ค์ ์ ๋์์ต๋๋ค. ์๋ก์ด ๋ผ์ด๋ธ๋ฌ๋ฆฌ ๋์ ๊ณผ Next.js ๋ฏธ๋ค์จ์ด ํ์ฉ์ ํตํด ์์ ์ ์ด๊ณ ์ฌ์ฉ์ ์นํ์ ์ธ ํ๊ฒฝ์ ์ ๊ณตํฉ๋๋ค. Highlights
๐ง New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with ๐ and ๐ on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces several new features related to user authentication, group management, and UI enhancements. It includes the implementation of a login modal, signup process, and various UI components for displaying book stories, news, and group information. The changes also incorporate new libraries for state management (Zustand), cookie handling (js-cookie), and non-blocking notifications (react-hot-toast). I have provided review comments to address potential issues related to date handling and code improvements.
| }: Props) { | ||
| return ( | ||
| <div className="flex h-[380px] w-[336px] flex-col overflow-hidden rounded-lg border-2 border-Subbrown-4 bg-White"> | ||
| {/* ์๋จ ํ๋กํ */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| fill | ||
| className="object-cover" | ||
| sizes="32px" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| {timeAgo(createdAt)} ์กฐํ์ {viewCount} | ||
| </p> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
๐ค Fix all issues with AI agents
In `@package.json`:
- Line 13: Update the Next.js dependency in package.json by replacing the
current "next": "16.0.1" entry with the patched version that addresses
CVE-2025-66478 (e.g., "next": "16.0.2" or the specific patched release from the
Next.js advisory); after changing the version string for the "next" dependency,
run your package manager (npm/yarn/pnpm) to install and regenerate lockfile to
ensure the patched version is applied.
In `@src/components/base-ui/Group-Search/search_club_apply_modal.tsx`:
- Line 169: The onClick currently calls onSubmit(reason) but onSubmit expects
(club: number, reason: string), so update the handler in
search_club_apply_modal.tsx to pass the club id first and the reason second
(e.g., onSubmit(club, reason) or onSubmit(clubId, reason) depending on the local
prop/name); ensure you reference the component's club identifier (the prop or
state variable used in this component) when invoking onSubmit so the parent
receives the correct club numeric id as the first argument.
In
`@src/components/base-ui/Group-Search/search_clublist/search_clublist_item.tsx`:
- Line 6: The ClubSummary interface currently declares a non-implemented method
reason(clubId: number, reason: string): void which no objects or code use;
remove that method signature from the ClubSummary interface declaration (the
ClubSummary interface in page.tsx) so the type matches the actual dummyClubs and
runtime objects and eliminate the unused contract.
๐ Major comments (26)
src/components/base-ui/News/recommendbook_element.tsx-44-57 (1)
44-57: Add accessible name + pressed state to the like toggle.Icon-only button is unlabeled for screen readers.
โฟ Suggested fix
<button type="button" onClick={(e) => { e.stopPropagation(); onLikeChange(!liked); }} className="w-[24px] h-[24px] shrink-0" + aria-label={liked ? 'Unlike' : 'Like'} + aria-pressed={liked} >src/components/base-ui/Search/search_bookresult.tsx-72-101 (1)
72-101: Icon-only buttons need accessible names (and pressed state for like).Screen readers get unlabeled controls. Add
aria-labelandaria-pressedfor the toggle.โฟ Suggested fix
<button type="button" onClick={(e) => { e.stopPropagation(); onLikeChange(!liked); }} className="w-[24px] h-[24px] shrink-0" + aria-label={liked ? 'Unlike' : 'Like'} + aria-pressed={liked} > <Image src={liked ? '/red_heart.svg' : '/gray_heart.svg'} alt="" width={24} height={24} /> </button> <button type="button" onClick={(e) => { e.stopPropagation(); onPencilClick?.(); }} className=" flex w-[60px] h-[60px] px-[10px] py-[4.167px] flex-col justify-center items-center gap-[8.333px] shrink-0 rounded-full bg-[color:var(--primary_2)] " + aria-label="Edit" > <Image src="/pencil_icon.svg" alt="" width={20} height={20} /> </button>src/components/base-ui/News/recommendbook_element.tsx-29-33 (1)
29-33: Make the clickable card keyboard-accessible.A clickable div without role/tabIndex and keyboard handling blocks keyboard-only users.
โฟ Suggested fix
<div - onClick={onCardClick} + onClick={onCardClick} + onKeyDown={(e) => { + if (!onCardClick) return; + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onCardClick(); + } + }} + role={onCardClick ? 'button' : undefined} + tabIndex={onCardClick ? 0 : undefined} className={`relative flex w-[244px] h-[320px] p-[12px] flex-col justify-end items-start gap-[10px] overflow-hidden ${ onCardClick ? 'cursor-pointer' : '' } ${className}`} >src/components/base-ui/Search/search_recommendbook.tsx-44-57 (1)
44-57: Add accessible name + pressed state to the like toggle.Icon-only button is unlabeled for screen readers.
โฟ Suggested fix
<button type="button" onClick={(e) => { e.stopPropagation(); onLikeChange(!liked); }} className="w-[24px] h-[24px] shrink-0" + aria-label={liked ? 'Unlike' : 'Like'} + aria-pressed={liked} >src/components/base-ui/Search/search_bookresult.tsx-37-43 (1)
37-43: Add keyboard access for the clickable card container.The div is clickable but not keyboard-focusable, blocking keyboard users. Use role/tabIndex and handle Enter/Space when
onCardClickis provided.โฟ Suggested fix
- <div - onClick={onCardClick} + <div + onClick={onCardClick} + onKeyDown={(e) => { + if (!onCardClick) return; + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onCardClick(); + } + }} + role={onCardClick ? 'button' : undefined} + tabIndex={onCardClick ? 0 : undefined} className={[ 'flex w-full max-w-[1040px] p-[20px] justify-center items-start gap-[24px] rounded-[8px] bg-[color:var(--White,`#FFF`)] shadow-[0_2px_4px_rgba(0,0,0,0.05)] border border-[color:var(--Subbrown_4,`#E0E0E0`)]', onCardClick ? 'cursor-pointer' : '', className, ].join(' ')} >src/components/base-ui/Search/search_recommendbook.tsx-29-33 (1)
29-33: Make the clickable card keyboard-accessible.A clickable div without role/tabIndex and keyboard handling blocks keyboard-only users.
โฟ Suggested fix
<div - onClick={onCardClick} + onClick={onCardClick} + onKeyDown={(e) => { + if (!onCardClick) return; + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onCardClick(); + } + }} + role={onCardClick ? 'button' : undefined} + tabIndex={onCardClick ? 0 : undefined} className={`relative flex w-[332px] h-[436px] p-[16px] flex-col justify-end items-start gap-[12px] overflow-hidden ${ onCardClick ? 'cursor-pointer' : '' } ${className}`} >src/lib/api/ApiError.ts-1-11 (1)
1-11: Replaceanywithunknownfor type safety.ESLint flags the use of
anyon lines 3 and 5. Usingunknownprovides better type safety while still allowing flexible response data.๐ง Proposed fix
export class ApiError extends Error { code: string; - response?: any; + response?: unknown; - constructor(message: string, code: string = "UNKNOWN_ERROR", response?: any) { + constructor(message: string, code: string = "UNKNOWN_ERROR", response?: unknown) { super(message); this.name = "ApiError"; this.code = code; this.response = response; } }src/components/base-ui/Join/steps/SignupComplete/useSignupComplete.ts-11-24 (1)
11-24: Remove debug artifacts before merging.The
console.logstatements in all handlers are debug artifacts. Additionally, the commented-outrouter.pushcalls inhandleSearchMeetingandhandleCreateMeetingsuggest incomplete implementation.๐งน Proposed cleanup
const handleSearchMeeting = () => { - console.log("Search Meeting clicked"); - // router.push('/meeting/search'); + router.push('/meeting/search'); }; const handleCreateMeeting = () => { - console.log("Create Meeting clicked"); - // router.push('/meeting/create'); + router.push('/meeting/create'); }; const handleUseWithoutMeeting = () => { - console.log("Use Without Meeting clicked"); router.push("/"); };src/components/base-ui/BookStory/bookstory_choosebook.tsx-1-2 (1)
1-2: Add'use client'directive for interactive component.This component has an
onClickhandler (line 57) which requires client-side JavaScript. Without the'use client'directive, the button click may not work as expected in Next.js App Router.๐ก Suggested fix
+'use client'; + import React from 'react'; import Image from 'next/image';src/components/base-ui/home/home_bookclub.tsx-32-32 (1)
32-32: Use Next.js Image component for consistency and optimization.Native
<img>is used here while the rest of the file usesnext/image. Also, the path should have a leading slash for proper public asset resolution.๐ก Suggested fix
- <img src="logo2.svg" alt="๋ก๊ณ " className="mx-auto mb-4 mt-[118px]" /> + <Image src="/logo2.svg" alt="๋ก๊ณ " width={100} height={100} className="mx-auto mb-4 mt-[118px]" />src/components/base-ui/Join/JoinInput.tsx-60-84 (1)
60-84: Associate the label with the input and label the toggle button.A
<span>doesnโt create an accessible label relationship, and the toggle button has no accessible name/state. Use a<label htmlFor>tied to the input and addaria-label/aria-pressedon the toggle. Ensure callers passidornamewhenlabelis provided.โฟ Proposed fix
-const JoinInput: React.FC<JoinInputProps> = ({ - label, - hideLabel, - className, - type, - ...props -}) => { +const JoinInput: React.FC<JoinInputProps> = ({ + label, + hideLabel, + className = "", + type, + ...props +}) => { + const inputId = props.id ?? props.name; const [showPassword, setShowPassword] = useState(false); const isPasswordType = type === "password"; const inputType = isPasswordType ? showPassword ? "text" : "password" : type; return ( <div className="flex flex-col items-start w-full gap-[12px]"> {label && ( - <span + <label + htmlFor={inputId} className={`text-[`#7B6154`] font-sans text-[20px] font-semibold leading-[135%] tracking-[-0.02px] ${ hideLabel ? "sr-only" : "" }`} > {label} - </span> + </label> )} <div className="relative w-full"> <input + id={inputId} type={inputType} className={`w-full h-[44px] px-[16px] py-[12px] bg-white border rounded-[8px] outline-none ${className} ${ isPasswordType ? "pr-[40px]" : "" }`} {...props} /> {isPasswordType && ( <button type="button" onClick={() => setShowPassword(!showPassword)} + aria-label={showPassword ? "๋น๋ฐ๋ฒํธ ์จ๊ธฐ๊ธฐ" : "๋น๋ฐ๋ฒํธ ํ์"} + aria-pressed={showPassword} className="absolute top-1/2 right-[12px] transform -translate-y-1/2 text-[`#BBB`] hover:text-[`#8D8D8D`]" > {showPassword ? <EyeOffIcon /> : <EyeIcon />} </button> )} </div> </div> ); };src/app/globals.css-1-1 (1)
1-1: Pin the Pretendard import to a specific version instead of@latest.Using
@latestcauses non-deterministic builds and can introduce unexpected breaking changes. Replace with the current stable version (v1.3.9):`@import` url("https://cdn.jsdelivr.net/gh/orioncactus/[email protected]/dist/web/static/pretendard.min.css");Consider using
.min.cssas shown above for better performance.src/utils/groupMapper.ts-1-1 (1)
1-1: Import types from the canonical source.Types are imported from
@/app/groups/pagebut should be imported from@/types/groups/groupswhereCategoryandParticipantTypeare canonically defined. Importing from a page component creates a coupling to that component and may cause circular dependency issues.๐ง Proposed fix
-import { Category, ParticipantType } from "@/app/groups/page"; +import { Category, ParticipantType } from "@/types/groups/groups";src/components/base-ui/Group-Search/search_groupsearch.tsx-6-13 (1)
6-13: DuplicateCategorytype definition.This
Categorytype is already defined insrc/types/groups/groups.ts. Duplicating type definitions can lead to inconsistencies if one is updated but not the other. Import from the canonical source instead.โป๏ธ Proposed fix
'use client'; import Image from 'next/image'; import { useEffect, useRef, useState } from 'react'; +import { Category } from '@/types/groups/groups'; -export type Category = - | '์ ์ฒด' - | '๋ํ์' - | '์ง์ฅ์ธ' - | '์จ๋ผ์ธ' - | '๋์๋ฆฌ' - | '๋ชจ์' - | '๋๋ฉด'; +// Re-export for consumers that import from this file +export type { Category } from '@/types/groups/groups';src/components/base-ui/Join/JoinLayout.tsx-13-15 (1)
13-15: Fixed width will break on mobile/tablet viewports.The inner container uses a fixed
w-[766px]width, which will cause horizontal overflow on screens smaller than 766px. Given the PR objectives mention responsive UI for tablet (768px) and mobile (375px), this needs responsive handling.๐ Suggested fix for responsive layout
- <div className="flex flex-col items-center w-[766px] px-[56px] py-[99px] gap-[100px] rounded-[8px] bg-White"> + <div className="flex flex-col items-center w-full max-w-[766px] px-4 sm:px-[56px] py-12 sm:py-[99px] gap-12 sm:gap-[100px] rounded-[8px] bg-White">src/lib/api/endpoints.ts-1-2 (1)
1-2: Production URL as fallback is risky for development.If
NEXT_PUBLIC_API_URLis not set (e.g., missing.env.local), development environments will silently hit the production API, potentially causing unintended data mutations or auth confusion.๐ง Safer alternatives
Option 1: Fail explicitly if env var is missing
-export const API_BASE_URL = - process.env.NEXT_PUBLIC_API_URL || "https://api.checkmo.co.kr/api"; +const envUrl = process.env.NEXT_PUBLIC_API_URL; +if (!envUrl) { + throw new Error("NEXT_PUBLIC_API_URL environment variable is not set"); +} +export const API_BASE_URL = envUrl;Option 2: Use localhost as safe fallback
export const API_BASE_URL = - process.env.NEXT_PUBLIC_API_URL || "https://api.checkmo.co.kr/api"; + process.env.NEXT_PUBLIC_API_URL || "http://localhost:3001/api";src/components/base-ui/Join/steps/PasswordEntry/usePasswordEntry.ts-6-15 (1)
6-15: DeriveisValiddirectly instead of usinguseEffect+useState.The static analysis correctly flags that calling
setStatewithin an effect for derived state causes unnecessary re-renders. SinceisValidis purely derived frompasswordandconfirmPassword, useuseMemoor compute it inline. Also,password.length > 0is redundant since the regex already requires 6+ characters.โป๏ธ Proposed fix using useMemo
-import { useState, useEffect } from "react"; +import { useState, useMemo } from "react"; export const usePasswordEntry = () => { const [password, setPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); - const [isValid, setIsValid] = useState(false); - useEffect(() => { - // 6-12์, ์๋ฌธ ์ต์ 1์, ํน์๋ฌธ์ ์ต์ 1์ - const passwordRegex = /^(?=.*[a-zA-Z])(?=.*[!@#$%^&*]).{6,12}$/; - const isPasswordValid = passwordRegex.test(password); - const isMatch = password === confirmPassword; - - setIsValid(isPasswordValid && isMatch && password.length > 0); - }, [password, confirmPassword]); + // 6-12์, ์๋ฌธ ์ต์ 1์, ํน์๋ฌธ์ ์ต์ 1์ + const isValid = useMemo(() => { + const passwordRegex = /^(?=.*[a-zA-Z])(?=.*[!@#$%^&*]).{6,12}$/; + return passwordRegex.test(password) && password === confirmPassword; + }, [password, confirmPassword]);src/app/groups/groupSearchDummy.ts-4-4 (1)
4-4:ClubSummaryinterface includes areasonmethod that dummy objects cannot satisfy.The
ClubSummaryinterface (page.tsx, lines 19-29) declaresreason(clubId: number, reason: string): void;as a method. ThedummyClubsarray in groupSearchDummy.ts defines plain object literals that omit this method, violating the interface contract.This is a design issueโthe
reasonmethod is never called on club objects. Instead,reasonis handled as a separate callback parameter in event handlers likeonSubmitApply(). Methods should not belong in data transfer objects. Remove thereasonmethod from the interface and pass the callback separately where needed, or create a separate handler type for apply actions.src/components/base-ui/Login/useLoginForm.tsx-78-82 (1)
78-82: Social login path is still a TODOThe PR objectives mention OAuth, but this handler only logs. Please implement or hide/disable the option until itโs ready.
If you want, I can draft the OAuth redirect flow or open a tracking issue.
src/components/base-ui/Join/steps/useEmailVerification.ts-36-40 (1)
36-40: Block verification after the timer expires
handleVerifyonly checks code length, so a user can still verify after timeout. Guard withtimeLeft.๐ง Suggested fix
const handleVerify = () => { - if (isCodeValid) { + if (isCodeValid && timeLeft !== null && timeLeft > 0) { setIsVerified(true); setShowToast(true); } };src/components/base-ui/Join/steps/ProfileSetup/ProfileSetup.tsx-39-45 (1)
39-45: Nickname becomes immutable after duplicate check
disabled={isNicknameChecked}prevents users from correcting typos, while the hookโs reset-on-change can never fire. This blocks signup if they want to edit.๐ง Suggested fix
- <JoinInput - value={nickname} - onChange={handleNicknameChange} - disabled={isNicknameChecked} + <JoinInput + value={nickname} + onChange={handleNicknameChange} placeholder="๋๋ค์์ ์ ๋ ฅํด์ฃผ์ธ์(์ต๋ 20๊ธ์)" className="border-[`#EAE5E2`] placeholder-[`#BBB`] text-[14px] font-normal" />src/components/base-ui/Join/steps/useEmailVerification.ts-13-18 (1)
13-18: Reset verification state when the email changesIf the user edits the email after verifying, the existing
isVerified/code/timer state carries over and can incorrectly mark a new email as verified.๐ง Suggested fix
const handleEmailChange = (e: React.ChangeEvent<HTMLInputElement>) => { const value = e.target.value; setEmail(value); const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; setIsEmailValid(emailRegex.test(value)); + setIsVerified(false); + setVerificationCode(""); + setIsCodeValid(false); + setTimeLeft(null); + setShowToast(false); };src/components/base-ui/Login/useLoginForm.tsx-47-65 (1)
47-65: Only proceed on true success and avoid leaking auth payloadsThe
authService.login()returns aLoginResponseobject without throwing on failure (indicated byisSuccess: false), so the try-catch does not intercept failed authentications. This means lines 59โ64 (state update, toast, and navigation) execute unconditionally even whenisSuccessis false. Additionally, line 49 logs the entire response payload, creating potential information leakage. The cookie also lacks an explicit root path.๐ง Suggested fix
// Service Layer ํธ์ถ const data = await authService.login(form); - - console.log("๋ก๊ทธ์ธ ์ฑ๊ณต:", data); - // 1. Token Storage (Secure Cookie) - if (data.isSuccess && data.result?.accessToken) { - Cookies.set("accessToken", data.result.accessToken, { - secure: true, - sameSite: "strict", - }); - } + if (!data.isSuccess || !data.result?.accessToken) { + throw new ApiError("LOGIN_FAILED", "LOGIN_FAILED", data); + } + + // 1. Token Storage (Secure Cookie) + Cookies.set("accessToken", data.result.accessToken, { + secure: true, + sameSite: "strict", + path: "/", + });src/app/groups/page.tsx-19-29 (1)
19-29:reasonshould not be a method signature in a data interface.The
ClubSummaryinterface definesreason(clubId: number, reason: string): voidwhich is a method signature. This appears to be a mistakeโdata interfaces should not contain callback methods. This will cause issues when creating objects that conform to this interface, as they would need to implement a method.Either remove this line or, if a reason field is needed, define it as a property:
๐ง Proposed fix
export interface ClubSummary { - reason(clubId: number, reason: string): void; clubId: number; name: string; profileImageUrl?: string | null; category: number[]; public: boolean; applytype: ApplyType; region: string; participantTypes: ParticipantType[]; }src/lib/api/client.ts-1-1 (1)
1-1:API_BASE_URLis imported but never used.The base URL is imported but not prepended to requests. Either remove the unused import or prepend it to
requestUrl.๐ง Proposed fix: prepend base URL
- let requestUrl = url; + let requestUrl = `${API_BASE_URL}${url}`;src/lib/api/client.ts-59-64 (1)
59-64: 401 handler does not interrupt the request flow.After detecting a 401, the code logs out and shows a toast but then continues to parse and potentially return the response. This can cause callers to receive and process an unauthorized response as valid data, leading to confusing behavior.
๐ง Proposed fix: throw after 401 handling
// [Resilience] Interceptor: 401 Unauthorized Handling if (response.status === 401) { console.warn("Session expired. Logging out..."); useAuthStore.getState().logout(); toast.error("์ธ์ ์ด ๋ง๋ฃ๋์์ต๋๋ค. ๋ค์ ๋ก๊ทธ์ธํด์ฃผ์ธ์."); - // ์ฌ๊ธฐ์ throw๋ฅผ ํด์ ํ๋ฆ์ ๋์ด์ฃผ๋ ๊ฒ์ด ์์ ํ ์ ์์ + throw new Error("Unauthorized: Session expired"); }
๐ก Minor comments (29)
tsconfig.json-35-39 (1)
35-39: Remove stalecheckmo/prefixed paths fromincludearray.The
includearray containscheckmo/.next/types/**/*.tsandcheckmo/.next/dev/types/**/*.ts(lines 35-36), but thecheckmo/directory does not exist in the repository. These are stale paths and should be removed. The.next/paths on lines 38-39 are correct for Next.js projects and should be kept (the.next/directory is generated at build time).Remove stale paths
"include": [ "next-env.d.ts", "**/*.ts", "**/*.tsx", - "checkmo/.next/types/**/*.ts", - "checkmo/.next/dev/types/**/*.ts", "**/*.mts", ".next/types/**/*.ts", ".next/dev/types/**/*.ts" ],src/components/base-ui/BookStory/bookstory_text.tsx-29-50 (1)
29-50: Accessibility concern: Tab key interception blocks keyboard navigation.Preventing the default Tab behavior means keyboard-only users cannot tab out of the textarea to reach other form elements. Consider allowing Tab navigation when no text is selected, or use a modifier key (e.g., Ctrl+Tab or Escape then Tab) for indentation.
๐ก Alternative: Only indent when there's a selection, otherwise allow normal Tab
const handleDetailKeyDown = useCallback( (e: React.KeyboardEvent<HTMLTextAreaElement>) => { if (e.key !== 'Tab') return; const el = e.currentTarget; const start = el.selectionStart ?? 0; const end = el.selectionEnd ?? 0; + // Allow normal tab navigation when no text is selected + if (start === end && !e.shiftKey) return; + e.preventDefault(); const insert = ' '; const next = detail.slice(0, start) + insert + detail.slice(end);src/components/base-ui/Search/search_bookresult.tsx-66-66 (1)
66-66: Fix likely class typo (flex1โflex-1).This appears to be a utility class typo and can break layout.
๐ฉน Suggested fix
- <p className="flex1 h-full text-[color:var(--Gray_4,`#8D8D8D`)] body_1_2 line-clamp-6"> + <p className="flex-1 h-full text-[color:var(--Gray_4,`#8D8D8D`)] body_1_2 line-clamp-6">src/components/base-ui/Join/steps/SignupComplete/useSignupComplete.ts-6-9 (1)
6-9: Replace hardcoded dummy data with actual user data.The hook uses hardcoded values instead of retrieving actual user data. This should integrate with the auth store or fetch user profile data after signup completion.
Would you like me to help integrate this with the Zustand auth store to retrieve actual user data?
src/components/common/Toast.tsx-24-31 (1)
24-31: Add ARIA live region attributes for screen-reader announcements.
Right now the toast wonโt be announced reliably by assistive tech.๐ ๏ธ Suggested fix
- <div + <div + role="status" + aria-live="polite" + aria-atomic="true" className={`absolute top-1/2 left-1/2 transform -translate-x-1/2 -translate-y-1/2 z-50 inline-flex justify-center items-center h-[88px] pl-[138px] pr-[137px] bg-[`#31111D99`] rounded-[24px] backdrop-blur-[1px] transition-opacity duration-300 ${ isVisible ? "opacity-100" : "opacity-0" }`} >src/components/base-ui/Profile/others_profile.tsx-88-100 (1)
88-100: Expose toggle state viaaria-pressed.
This is a toggle button, so assistive tech should get the pressed state.๐ ๏ธ Suggested fix
- <button + <button type="button" onClick={() => onToggleSubscribe(!isSubscribed)} + aria-pressed={isSubscribed} className={[ 'flex w-[532px] h-[48px] px-[16px] py-[12px] justify-center items-center gap-[10px] rounded-[8px]', 'subhead_4_1 whitespace-nowrap', isSubscribed ? 'bg-[color:var(--Subbrown_4,`#EAE5E2`)] text-[color:var(--primary_3,`#5E4A40`)]' : 'bg-[color:var(--Primary_1,`#7B6154`)] text-[color:var(--White,`#FFF`)]', ].join(' ')} >src/components/base-ui/Join/steps/SignupComplete/SignupComplete.tsx-35-41 (1)
35-41: Use a descriptive alt for the profile image.
Helps accessibility and aligns with user-specific content.๐ ๏ธ Suggested fix
- <Image - src={profileImage} - alt="Profile" + <Image + src={profileImage} + alt={`${nickname} ํ๋กํ`} width={138} height={138} className="object-cover w-full h-full" />src/components/base-ui/home/list_subscribe.tsx-25-25 (1)
25-25: Removeconsole.logbefore production.Debug logging should not remain in production code. Replace with actual subscription logic or a no-op placeholder.
๐ก Suggested fix
- onSubscribeClick={() => console.log('subscribe', u.id)} + onSubscribeClick={() => { + // TODO: Implement subscription logic + }}src/components/base-ui/home/home_bookclub.tsx-14-19 (1)
14-19: Inconsistent threshold vs. preview count.The component triggers collapse mode when
count >= 5, but then displays 6 items when collapsed. This creates an edge case where having exactly 5 groups would show 5 items with a "์ ์ฒด๋ณด๊ธฐ" toggle that does nothing meaningful.Consider aligning these values:
๐ก Suggested fix
- const isMany = count >= 5; + const isMany = count > 6; const [open, setOpen] = useState(false); // ์ ํ: 6๊ฐ๋ง / ํผ์นจ: ์ ์ฒด const displayGroups = isMany && !open ? groups.slice(0, 6) : groups;src/components/base-ui/home/list_subscribe_element.tsx-24-32 (1)
24-32: Mismatchedsizesprop value.The Image container is
32x32pxbutsizes="42px"is specified. This should match the actual rendered size for optimal image loading.๐ก Suggested fix
<Image src={profileSrc} alt={`${name} profile`} fill className="object-cover" - sizes="42px" + sizes="32px" priority={false} />src/components/base-ui/BookStory/bookstory_card.tsx-18-30 (1)
18-30: Add validation for invalid date strings.The
timeAgofunction doesn't handle invalid ISO strings, which would causenew Date(iso)to returnInvalid Dateand result inNaN-based calculations returning unexpected output like "NaN์ผ ์ ".๐ก Suggested defensive fix
function timeAgo(iso: string) { + const date = new Date(iso); + if (isNaN(date.getTime())) return ''; - const diff = Date.now() - new Date(iso).getTime(); + const diff = Date.now() - date.getTime(); const minutes = Math.floor(diff / 60000);src/components/base-ui/Group-Search/search_mybookclub.tsx-69-85 (1)
69-85: Conflicting CSS classes:gridandflex-col.Line 71 combines
grid grid-cols-1withflex-col. These are mutually exclusive layout modesโflex-colhas no effect whendisplay: gridis applied. Removeflex-col.๐ง Proposed fix
className={[ - "grid grid-cols-1 t:grid-cols-2 d:grid-cols-1 flex-col gap-2", + "grid grid-cols-1 t:grid-cols-2 d:grid-cols-1 gap-2", open && showToggle ? "overflow-y-auto pr-1" : "", ].join(" ")}src/components/base-ui/Group-Search/search_mybookclub.tsx-62-65 (1)
62-65: Empty Tailwind classh-[]appears to be incomplete.The
h-[]class on line 62 is an empty arbitrary value that has no effect. This looks like incomplete code or a typo. Either remove it or specify an intended height value.๐ง Proposed fix
- <div className="h-[] flex items-center justify-center py-4 t:py-10 d:py-20"> + <div className="flex items-center justify-center py-4 t:py-10 d:py-20">src/components/auth/AuthProvider.tsx-10-21 (1)
10-21: Token hydration sets incomplete user state without validation.The current implementation trusts the cookie token existence without validating it server-side. This means:
- An expired or tampered token will set
isLoggedIn: trueuntil the first API call failsuser.emailis empty, which may cause issues if other components expect it whenisLoggedInis trueThe TODO comment indicates this is intentional for now, but consider prioritizing the
/api/auth/mecall to validate the token and fetch complete user data on hydration.Would you like me to help implement the token validation flow using
/api/auth/me?src/components/base-ui/Login/LoginModal.tsx-58-69 (1)
58-69: Email input should usetype="email".Using
type="text"loses browser validation, mobile keyboard optimization, and autocomplete capabilities for email fields.Proposed fix
<input name="email" - type="text" + type="email" value={form.email} onChange={handleChange} placeholder="์ด๋ฉ์ผ"src/app/groups/groupSearchDummy.ts-7-17 (1)
7-17: Duplicate IDs in dummy data will cause React key collisions.The
mydummyGrouparray contains duplicateidvalues ('1', '2', '3', '4' each appear twice). When this data is rendered withidas a React key, you'll get key collision warnings and potential rendering bugs.Proposed fix
export const mydummyGroup: GroupSummary[] = [ { id: '1', name: '๋ชจ์1' }, { id: '2', name: '๋ชจ์2' }, { id: '3', name: '๋ชจ์3' }, { id: '4', name: '๋ชจ์4' }, - { id: '1', name: '๋ชจ์11241' }, - { id: '2', name: '๋ชจ์51212' }, - { id: '3', name: '๋ชจ์125153' }, - { id: '4', name: '๋ชจ์12512514' }, + { id: '5', name: '๋ชจ์11241' }, + { id: '6', name: '๋ชจ์51212' }, + { id: '7', name: '๋ชจ์125153' }, + { id: '8', name: '๋ชจ์12512514' }, ];src/components/base-ui/home/NewsBannerSlider.tsx-17-17 (1)
17-17: Fixed dimensions break responsiveness.The container uses hardcoded
h-[424px] w-[1040px], which won't adapt to tablet (768px) or mobile (375px) breakpoints mentioned in the PR objectives.Suggested responsive approach
- <div className="relative h-[424px] w-[1040px] overflow-hidden rounded-[10px]"> + <div className="relative aspect-[1040/424] w-full max-w-[1040px] overflow-hidden rounded-[10px]">src/components/base-ui/Login/LoginModal.tsx-136-141 (1)
136-141: Same accessibility issue: "ํ์๊ฐ์ ํ๋ฌ๊ฐ๊ธฐ" should be a button.The signup link in the footer has the same accessibility problem as the find account links.
Proposed fix
<p className={styles.footerText}> ์์ง ํ์์ด ์๋์ ๊ฐ์?{" "} - <span className={styles.footerLink} onClick={onSignUp}> + <button type="button" className={styles.footerLink} onClick={onSignUp}> ํ์๊ฐ์ ํ๋ฌ๊ฐ๊ธฐ - </span> + </button> </p>src/components/base-ui/Login/LoginModal.tsx-91-99 (1)
91-99: Interactive<span>elements are not keyboard accessible.Using
<span>withonClickfor "์์ด๋ ์ฐพ๊ธฐ" and "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ" lacks keyboard support (no focus, no Enter/Space activation). Use<button>elements instead.Proposed fix
<div className={styles.findAccount}> - <span className={styles.link} onClick={onFindAccount}> + <button type="button" className={styles.link} onClick={onFindAccount}> ์์ด๋ ์ฐพ๊ธฐ - </span> + </button> <span className={styles.divider}>|</span> - <span className={styles.link} onClick={onFindAccount}> + <button type="button" className={styles.link} onClick={onFindAccount}> ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ - </span> + </button> </div>src/app/(main)/page.tsx-23-25 (1)
23-25: LoginModal is missingonFindAccountandonSignUphandlers.According to the
LoginModalcomponent signature, it acceptsonFindAccountandonSignUpoptional props. Currently, clicking "์์ด๋ ์ฐพ๊ธฐ", "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", or "ํ์๊ฐ์ ํ๋ฌ๊ฐ๊ธฐ" links will have no effect since these handlers aren't provided.Consider wiring up placeholder handlers or disabling those UI elements until the functionality is implemented.
src/components/base-ui/Join/JoinButton.tsx-18-23 (1)
18-23: Secondary variant lacks disabled styling.The
primaryvariant has explicit disabled styling, butsecondarydoes not. A disabled secondary button will remain visually unchanged, which may confuse users.๐ Proposed fix
const variants = { primary: disabled ? "bg-[`#DADADA`] text-[`#8D8D8D`] cursor-not-allowed" : "bg-[`#7B6154`] text-[`#FFF`]", - secondary: "bg-[`#EAE5E2`] text-[`#5E4A40`] border border-[`#D2C5B6`]", + secondary: disabled + ? "bg-[`#F5F5F5`] text-[`#BBBBBB`] border border-[`#E0E0E0`] cursor-not-allowed" + : "bg-[`#EAE5E2`] text-[`#5E4A40`] border border-[`#D2C5B6`]", };src/components/base-ui/Group-Search/search_clublist/search_club_category_tags.tsx-50-56 (1)
50-56: Typo:item-centershould beitems-center.Line 51 has a typo in the Tailwind class name.
item-centeris not a valid Tailwind utility; it should beitems-center.๐ Proposed fix
<span key={n} className={[ - 'h-[21px] my-auto py-[1px] inline-flex item-center justify-center body_1_2', + 'h-[21px] my-auto py-[1px] inline-flex items-center justify-center body_1_2', 'rounded-[8px] text-White', short ? 'w-[44px]' : 'px-2', getBgByCategory(n), className, ].join(' ')} >src/components/base-ui/Join/steps/ProfileImage/InterestCategorySelector.tsx-29-33 (1)
29-33: Add explicit button type to prevent future form submission issuesThe button currently lacks an explicit
typeattribute. While the component is not currently rendered inside a<form>, addingtype="button"is a best practice to prevent accidental form submission if the component is ever used in a form context in the future.๐ง Suggested fix
<button key={category} + type="button" + aria-pressed={isSelected} onClick={() => onToggle(category)} className={`w-[122px] h-[44px] flex justify-center items-center rounded-[400px] text-[14px] leading-[145%] tracking-[-0.014px] transition-colors ${src/components/base-ui/Join/steps/TermsItem.tsx-33-41 (1)
33-41: Make checkbox icons decorative for screen readersThe label already conveys state; these images should be marked as decorative using an empty
alt=""attribute to prevent screen readers from announcing them.๐ง Suggested fix
- <Image - src="/CheckBox_No.svg" - alt="Unchecked" - width={24} - height={24} - /> + <Image + src="/CheckBox_No.svg" + alt="" + width={24} + height={24} + /> ... - <Image src="/CheckBox_Yes.svg" alt="Checked" width={24} height={24} /> + <Image + src="/CheckBox_Yes.svg" + alt="" + width={24} + height={24} + />src/app/groups/create/page.tsx-411-416 (1)
411-416: MissingmaxLengthattribute for activity area input.Similar to the description textarea, this input's placeholder says "40์ ์ ํ" but no
maxLength={40}is set.๐ง Proposed fix
<input value={activityArea} onChange={(e) => setActivityArea(e.target.value)} placeholder="ํ๋ ์ง์ญ์ ์ ๋ ฅํด์ฃผ์ธ์ (40์ ์ ํ)" + maxLength={40} className="..." />src/components/base-ui/Join/steps/EmailVerification/EmailVerification.tsx-30-30 (1)
30-30: Fixed width may break on mobile viewports.The container uses
w-[766px]which exceeds mobile viewport width (375px per PR objectives). Consider using responsive classes likew-full max-w-[766px]or adding breakpoint variants.๐ง Suggested responsive fix
- <div className="relative flex flex-col items-center w-[766px] px-[56px] py-[99px] bg-white rounded-[8px]"> + <div className="relative flex flex-col items-center w-full max-w-[766px] px-4 sm:px-[56px] py-10 sm:py-[99px] bg-white rounded-[8px]">src/app/groups/create/page.tsx-195-216 (1)
195-216: MissingmaxLengthattribute despite placeholder indicating limit.The placeholder text states "500์ ์ ํ" but there's no
maxLength={500}attribute to enforce the limit. Users can currently enter unlimited text.๐ง Proposed fix
<textarea value={clubDescription} onChange={(e) => { setClubDescription(e.target.value); autoResize(e.currentTarget); }} onInput={(e) => autoResize(e.currentTarget)} placeholder="์์ ๋กญ๊ฒ ์ ๋ ฅํด์ฃผ์ธ์! (500์ ์ ํ)" + maxLength={500} className="..." />src/app/groups/create/page.tsx-473-539 (1)
473-539: Avoid using array index askeyfor dynamic lists.Using
key={idx}for the links list can cause incorrect React reconciliation when rows are removed. If the user deletes the first row, the second row's state may not update correctly.๐ง Proposed fix: use a stable unique ID
-type SnsLink = { label: string; url: string }; +type SnsLink = { id: number; label: string; url: string }; +const nextLinkId = useRef(1); -const [links, setLinks] = useState<SnsLink[]>([{ label: "", url: "" }]); +const [links, setLinks] = useState<SnsLink[]>([{ id: 0, label: "", url: "" }]); const addLinkRow = () => { - setLinks((prev) => [...prev, { label: "", url: "" }]); + setLinks((prev) => [...prev, { id: nextLinkId.current++, label: "", url: "" }]); }; // Then in JSX: -{links.map((it, idx) => ( - <div key={idx} ...> +{links.map((it) => ( + <div key={it.id} ...>src/components/base-ui/Join/steps/ProfileImage/useProfileImage.ts-49-56 (1)
49-56: Memory leak: old blob URL not revoked when replaced.The cleanup function only runs on unmount (or when
profileImagechanges), but at that point it revokes the new value. When a user uploads a second image, the previous blob URL is never revoked.๐ง Proposed fix using a ref to track the previous blob URL
-import { useState, useEffect } from "react"; +import { useState, useEffect, useRef } from "react"; ... + const prevBlobRef = useRef<string | null>(null); + const handleImageUpload = (e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0]; if (file) { const imageUrl = URL.createObjectURL(file); setProfileImage(imageUrl); } }; // ๋ฉ๋ชจ๋ฆฌ ๋์ ๋ฐฉ์ง๋ฅผ ์ํ cleanup useEffect(() => { + // Revoke previous blob URL when profileImage changes + if (prevBlobRef.current && prevBlobRef.current.startsWith("blob:")) { + URL.revokeObjectURL(prevBlobRef.current); + } + prevBlobRef.current = profileImage; + return () => { if (profileImage && profileImage.startsWith("blob:")) { URL.revokeObjectURL(profileImage); } }; }, [profileImage]);
๐ก To Reviewers
ํด๋น ๋ธ๋์น์์ ์๋กญ๊ฒ ์ค์นํ ๋ผ์ด๋ธ๋ฌ๋ฆฌ๊ฐ ์๋ค๋ฉด ํจ๊ป ๋ช ์ํด ์ฃผ์ธ์.
์๋กญ๊ฒ ์ค์นํ ๋ผ์ด๋ธ๋ฌ๋ฆฌ: zustand, js-cookie, react-hot-toast
๋ฆฌ๋ทฐ์ด๊ฐ ์ฝ๋๋ฅผ ์ดํดํ๋ ๋ฐ ๋์์ด ๋๋ ์ ๋ณด๋ ์ฐธ๊ณ ์ฌํญ์ด ์๋ค๋ฉด ์์ ๋กญ๊ฒ ์์ฑํด ์ฃผ์ธ์.
์ํคํ ์ฒ ์ฐธ๊ณ ์ฌํญ:
๐ฅ ์์ ๋ด์ฉ (๊ฐ๋ฅํ ๊ตฌ์ฒด์ ์ผ๋ก ์์ฑํด ์ฃผ์ธ์)
๋ก๊ทธ์ธ ๋ชจ๋ฌ ๋ฐ ๋ฐ์ํ UI ๊ตฌํ
์ธ์ฆ ์์คํ ์ธํ๋ผ ๊ตฌ์ถ:
UX ํผ๋๋ฐฑ ์์คํ :
๐ค ์ถํ ์์ ์์
๐ธ ์์ ๊ฒฐ๊ณผ (์คํฌ๋ฆฐ์ท)
๐ ๊ด๋ จ ์ด์
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes & Improvements
Chores
โ๏ธ Tip: You can customize this high-level summary in your review settings.